Skip to content

Document compat. guarantees, monitor serialization compat#931

Merged
tnull merged 3 commits into
lightningdevkit:mainfrom
tnull:2026-06-compat-guarantees-downgrade-070
Jun 11, 2026
Merged

Document compat. guarantees, monitor serialization compat#931
tnull merged 3 commits into
lightningdevkit:mainfrom
tnull:2026-06-compat-guarantees-downgrade-070

Conversation

@tnull

@tnull tnull commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #74.

We briefly document our compat guarantees in README.md.

We also add a canary test that checks whether we're forwards compatible with v0.7.0 on the serialization layer. Note that due to the recent schema upgrades of SqliteStore, FilesystemStore, VssStore we don't actually test full downgrades to prior versions. However, this canary is meant to be extended going forward, so that hopefully at some point we can be comfortable to guarantee forward compatibility guarantees also.

@tnull tnull requested a review from joostjager June 11, 2026 09:31
@ldk-reviews-bot

ldk-reviews-bot commented Jun 11, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @joostjager as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tnull tnull self-assigned this Jun 11, 2026
@tnull tnull moved this to Goal: Merge in Weekly Goals Jun 11, 2026
@tnull tnull added this to the 0.8 milestone Jun 11, 2026
&esplora_url,
);

assert_eq!(node_a_v070.node_id(), node_id_a);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe recheck to see if the payment is still there?

Comment thread README.md Outdated

## Compatibility

LDK Node does not provide a stable public API until v1.0. We do aim to keep persisted node state backwards compatible, so newer releases are guaranteed to be able to load state written by older releases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads like an aspiration (aim) mixed with a guarantee. Is it a guarantee?

Perhaps also state explicitly that downgrades are not supported.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point. Codex first take was too lax and I only amended the second part of the sentence. Now added a fixup.


async fn drain_v070_events(node: &ldk_node_070::Node) {
while tokio::time::timeout(Duration::from_millis(250), node.next_event_async()).await.is_ok() {
node.event_handled().unwrap();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if you want to do any kind of matching on types here. Maybe an error surfaces here?

@tnull tnull Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow, can you reformulate your question?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean events are drained without looking at them, and I was wondering if a downgrade problem could surface there too (and is currently ignored).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, well, I think we have expect calls for the event types we want to check already, and for the rest we just ignore? Do you have any particular checks in mind that we'd should still be doing?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, nothing in mind. Just posing the question whether this is a way to detect problems. If not, also fine.

@tnull tnull force-pushed the 2026-06-compat-guarantees-downgrade-070 branch from 69c4d3b to 6741c9d Compare June 11, 2026 12:43
@tnull tnull requested a review from joostjager June 11, 2026 12:43
joostjager
joostjager previously approved these changes Jun 11, 2026
@tnull tnull force-pushed the 2026-06-compat-guarantees-downgrade-070 branch from 6741c9d to 1f790a9 Compare June 11, 2026 13:36
@tnull tnull requested a review from joostjager June 11, 2026 13:39
@tnull

tnull commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Squashed fixups, and included minor change to account for uniffi API differences in tests.

tnull added 3 commits June 11, 2026 16:35
Clarify that public APIs remain unstable before 1.0 while persisted
node state is intended to remain readable by newer releases.

Co-Authored-By: HAL 9000
Add a downgrade canary that writes current node state through the legacy
v1 filesystem store and reopens it with ldk-node v0.7.0. This monitors
whether serialized node, channel, and payment state remains usable by
v0.7.0, including a restored channel and a post-restart payment.

This does not assert that the current filesystem-store v2 IO layout can
downgrade to v0.7.0's v1 layout. That IO-layer downgrade is unsupported:
v2 stores empty namespaces under [empty], which v1 readers do not look
up.

Co-Authored-By: HAL 9000
@tnull tnull force-pushed the 2026-06-compat-guarantees-downgrade-070 branch from 1f790a9 to a3a7606 Compare June 11, 2026 14:35
@tnull

tnull commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Accidentally rebased before (now reverted). Net diff since last push is:

> git diff-tree -U2 6741c a3a76061
diff --git a/tests/upgrade_downgrade_tests.rs b/tests/upgrade_downgrade_tests.rs
index dbfcf7c0..b30b5a33 100644
--- a/tests/upgrade_downgrade_tests.rs
+++ b/tests/upgrade_downgrade_tests.rs
@@ -186,4 +186,5 @@ fn build_current_node(
 	let mut fs_store_path = PathBuf::from(&config.storage_dir_path);
 	fs_store_path.push("fs_store");
+	#[allow(unused_mut)]
 	let mut builder = ldk_node::Builder::from_config(config);
 	builder.set_node_alias(alias.to_string()).unwrap();
@@ -244,5 +245,8 @@ async fn send_current_bolt11_payment(
 		CurrentDescription::new(description.to_owned()).unwrap(),
 	);
-	let invoice = payee.bolt11_payment().receive(amount_msat, &invoice_description, 3600).unwrap();
+	let invoice = payee
+		.bolt11_payment()
+		.receive(amount_msat, &invoice_description.clone().into(), 3600)
+		.unwrap();
 	let payment_id = payer.bolt11_payment().send(&invoice, None).unwrap();
 	expect_current_payment_successful(payer, &payment_id).await;

@joostjager joostjager left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't verify whether uniffi tests pass locally now.

@tnull tnull merged commit 010b483 into lightningdevkit:main Jun 11, 2026
@github-project-automation github-project-automation Bot moved this from Goal: Merge to Done in Weekly Goals Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Document compat. guarantees

3 participants